Resolve dbg linker problems due to asserts#10109
Conversation
libstdc++ and libc++ differ and abc uses asserts. It is compiled with libstdc++ headers and then linked with libc++ without this change. Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
There was a problem hiding this comment.
Code Review
This pull request modifies the .bazelrc file to force the use of libc++ for the abc external dependency, addressing linker errors encountered during debug builds. The reviewer suggests that configuring the standard library globally within the toolchain definition would be a more robust solution to prevent potential ODR violations and also identified a minor typo in the documentation comments.
| # This avoid problems with building -c dbg which enables asserts. | ||
| # Without this libstdc++ vs libc++ differences cause link errors. | ||
| build --per_file_copt=.*external/abc.*@-stdlib=libc++ | ||
| build --host_per_file_copt=.*external/abc.*@-stdlib=libc++ |
There was a problem hiding this comment.
Forcing libc++ for a specific external dependency via per_file_copt is a workaround for a broader inconsistency. If the project is intended to use libc++ globally (as suggested by the linker errors and the -libcxx_assertions feature in BUILD.bazel), it is more robust to configure this in the toolchain definition within MODULE.bazel using the stdlib = "libc++" attribute on the llvm.toolchain extension. This ensures consistency across all dependencies and avoids potential ODR violations that can occur when mixing standard library implementations. Additionally, there is a small typo in the comment ("avoid" should be "avoids").
# This avoids problems with building -c dbg which enables asserts.
# Without this libstdc++ vs libc++ differences cause link errors.
build --per_file_copt=.*external/abc.*@-stdlib=libc++
build --host_per_file_copt=.*external/abc.*@-stdlib=libc++
|
clang-tidy review says "All clean, LGTM! 👍" |
|
@hzeller @QuantamHD merging to unblock but also interested in your opinions on the right way to handle this. The summary of a tedious debug session |
|
Based on gemini's suggestion claude came back with I haven't tried it yet. |
|
Since we use Not sure how related, but there are a few files in abc that are c++ though and there is a C-file that needed to be renamed to c++ for the atomic include; @QuantamHD fixed that and I have applied it to the latest yosys in BCR bazelbuild/bazel-central-registry#8356 - So independent of if this is related, updating the bazel dep to I wonder why the c++ library linking is not working as expected. This PR fixes a problem, but for me to understand, how can the problem that shows up without the change be reproduced ? |
Summary
libstdc++ and libc++ differ and abc uses asserts. It is compiled with libstdc++ headers and then linked with libc++ without this change.
Type of Change
Verification
./etc/Build.sh).